Defensively thread path into NestedTypeVarSubstitutionRepairVisitor - #1683
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Walkthrough
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java (1)
97-109: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd Javadoc for the private constructor.
This constructor now selects the invocation path and rebuilds
VisitorState. Document the nullableinvocationPathparameter, the fallback tostate.getPath(), and that the stored path ends atinvocationTree.As per coding guidelines,
**/src/main/**/*.java: Add Javadoc for every non-trivial method, including private methods.Proposed Javadoc
+ /** + * Creates a repair visitor with a state path that ends at {`@code` invocationTree}. + * + * <p>If {`@code` invocationPath} is {`@code` null}, derives the path from {`@code` state.getPath()}. + */ private NestedTypeVarSubstitutionRepairVisitor(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java` around lines 97 - 109, Add Javadoc immediately above the private constructor containing the shown initialization, documenting the nullable invocationPath parameter, its fallback to state.getPath(), and that the rebuilt VisitorState path ends at invocationTree.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@nullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java`:
- Around line 97-109: Add Javadoc immediately above the private constructor
containing the shown initialization, documenting the nullable invocationPath
parameter, its fallback to state.getPath(), and that the rebuilt VisitorState
path ends at invocationTree.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c0fc3711-f555-40fb-9526-f5d952180119
📒 Files selected for processing (2)
nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.javanullaway/src/main/java/com/uber/nullaway/generics/NestedTypeVarSubstitutionRepairVisitor.java
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1683 +/- ##
=========================================
Coverage 87.77% 87.77%
- Complexity 3142 3143 +1
=========================================
Files 109 109
Lines 10606 10608 +2
Branches 2150 2151 +1
=========================================
+ Hits 9309 9311 +2
Misses 611 611
Partials 686 686 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
9b2b56b to
d0a83c4
Compare
d0a83c4 to
6fc043b
Compare
This addresses step 2 of #1680 (comment)
Summary by CodeRabbit